fix(sdk): revert agent-client-protocol to optional [acp] extra#2367
Open
simonrosenberg wants to merge 1 commit intomainfrom
Open
fix(sdk): revert agent-client-protocol to optional [acp] extra#2367simonrosenberg wants to merge 1 commit intomainfrom
simonrosenberg wants to merge 1 commit intomainfrom
Conversation
Move agent-client-protocol from a hard required dependency back to an optional extra (pip install openhands-sdk[acp]). This unblocks downstream consumers (e.g. the CLI) that pin ACP <0.8.0 from upgrading to SDK v1.12.0+. Changes: - pyproject.toml: move agent-client-protocol to [project.optional-dependencies] acp - __init__.py: catch ImportError in lazy __getattr__ with helpful message - test_acp_agent.py: skip entire module when acp not installed - example: add install prerequisite note - root pyproject.toml: add agent-client-protocol to dev deps for CI Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
API breakage checks (Griffe)Result: Passed |
Contributor
Agent server REST API breakage checks (OpenAPI)Result: Failed Log excerpt (first 1000 characters) |
Collaborator
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Acceptable - Pragmatic solution to a real version conflict problem.
This change makes agent-client-protocol optional, unblocking the CLI from upgrading. The implementation is straightforward:
- ✅ Solves real problem (CLI pinned to
<0.8.0, SDK now requires>=0.8.1) - ✅ Clear error message when dependency missing
- ✅ Proper test handling with
pytest.importorskip - ✅ Dev dependencies updated so CI tests still run
Missing Evidence
Your test plan has two unchecked manual verification steps. Add an Evidence section to the PR description showing:
-
SDK installs without ACP:
pip install openhands-sdk pip list | grep agent-client-protocol # should be empty
-
Import error is clear:
python -c "from openhands.sdk.agent import ACPAgent" # should show: "The 'agent-client-protocol' package is required..."
Tests passing is good, but for packaging changes, seeing the actual install/import behavior confirms it works as intended.
Verdict: Code is correct and ready to merge once evidence is added. 🚢
Key Insight: This is dependency management pragmatism - not everyone needs every feature, and optional extras exist to prevent dependency conflicts. The implementation is sound.
Contributor
Coverage Report •
|
||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
agent-client-protocolfrom a hard required dependency back to an optional extra (pip install openhands-sdk[acp])agent-client-protocol>=0.8.1, unblocking downstream repos (e.g. the CLI) that pinagent-client-protocol<0.8.0__getattr__import in__init__.pynow catchesImportErrorand provides a clear install instructionContext
Commit
e9cbac0(Feb 20) promotedagent-client-protocolfrom an[acp]optional extra to a hard required dependency. This created a version conflict: the CLI pinsagent-client-protocol>=0.7.0,<0.8.0while the SDK now requires>=0.8.1. These constraints are mutually exclusive, preventing the CLI from upgrading to SDK v1.12.0+.Changes
openhands-sdk/pyproject.tomlagent-client-protocol>=0.8.1fromdependenciesto[project.optional-dependencies] acpopenhands-sdk/.../agent/__init__.pyImportErrorin__getattr__with helpful install messagetests/sdk/agent/test_acp_agent.pypytest.importorskip("acp")to skip when not installedexamples/.../40_acp_agent_example.pypyproject.toml(root)agent-client-protocolto dev deps so CI tests still runuv.lockTest plan
pytest tests/sdk/agent/test_acp_agent.py)pytest tests/sdk/agent/)pip install openhands-sdkshould succeed withoutagent-client-protocolfrom openhands.sdk.agent import ACPAgentraises clearImportErrorwhen ACP not installed🤖 Generated with Claude Code